Repository navigation
Conversation
The installer existed with nothing to drive it. This adds the client the release plan's updater boundary described, meeting the conditions it set: signature, publisher and embedded version all verified before elevation, and the /D= target computed from the running executable rather than read from the uninstall registry key, which anything running as the user can write. GitHub Releases is the feed, so there is no manifest server to run and no second place a version is recorded. A tag that is not a version this project publishes is refused rather than string-compared, and the asset has to be named for the version the tag claims, so a file attached beside the real installer cannot be served in its place. An updater executes code that arrived over the network, with elevation, so what it refuses matters more than how it installs. The host allowlist is applied after redirects, because that is where the host can change. The size ceiling holds against Content-Length and again while the bytes arrive, since a hostile server owes us no honest header. The digest is taken while streaming and compared again before launch, closing the gap in which anything able to write to the download directory could swap the file. Authenticode must report Valid, with this project's thumbprint and publisher name, and the installer's own embedded FileVersion must match the release: without that last check, re-uploading an older but still validly signed installer under a newer asset name would roll every install back onto a build whose faults are already fixed. Two things the reference implementation does are deliberately not carried over. It terminates the running application before installing, which would contradict the promise that maintenance never force-kills a copy mid-transfer; this verifies and hands over instead, and says so before the elevation prompt so the installer's refusal does not read as a broken update. And it rejects any tag that is not X.Y.Z, which would make the updater blind to Offloader's own first packaged release. The grammar here is the one versioning.py already enforces, with a test asserting both orderings agree, because a disagreement would let an update install a build Windows then considers older. The in-app check, notification and progress surface stay deferred.
The assertion hardcoded a Windows path, and a backslash is not a separator off Windows: install_target resolves the running executable, so on Linux and macOS the literal resolved against the working directory and the test failed everywhere except where it was written. The property it checks is platform-independent, so it now uses a real directory and the fallback is asserted separately.
owenpkent
left a comment
owenpkent
left a comment
There was a problem hiding this comment.
Reviewed feed discovery, version/asset selection, download handling, installer verification, installed-location handling, and the CLI handoff. The actual feed cannot deliver the planned beta-to-beta updates; details inline.
Additional updater-security findings are supplied privately under SECURITY.md, including their affected call paths and suggested regression coverage. This public comment omits those reproductions. Validation for this review was static code inspection and verification of GitHub's documented release-feed behavior; no downloaded installer was executed.
/releases/latest is documented as returning the newest published full release and excluding prereleases. Every release this project has planned so far is a prerelease: the candidate workflow publishes with --prerelease, and the first packaged build is 0.1.0b1. So an installed 0.1.0b1 could not discover 0.1.0b2 or 0.1.0rc1 through that endpoint, and a repository holding only betas answered with nothing at all. The module already went out of its way not to reject prereleases the way Alpha-OSK's updater does, and then asked an endpoint that had already dropped them. The feed is the collection now. It is ordered by creation date rather than by version, so every entry is read and the greatest eligible one wins: a patched 0.1.0b2 published after 0.1.0rc1 is listed above it without being above it. Drafts are skipped, because a draft is visible to anyone who can write to the repository and its assets are not published, so offering one would hand an installer to this machine before the release exists for anybody else. Which releases an install is offered is now stated rather than implied. Newer, and on the channel it is already on: a build that is itself a prerelease is testing the prereleases and takes the next one, and a stable install is offered only stable releases. 0.1.0b1 finding 0.1.0b2 must not also mean 1.0.0 finding 1.0.1b1. A prerelease install still takes the stable release when it arrives, because 0.1.0b2 < 0.1.0 in the existing ordering, which is how a beta tester ends up on the shipping build without doing anything. Changing FEED_URL is free today and would not be later: it is compiled into every shipped build, and nothing has shipped. Eight feed-level tests against a collection holding betas, a release candidate, a stable release and a draft, rather than version comparison against one hand-picked payload. One of them covers an unusable entry not hiding the rest of the feed. docs/updates.md updated to match.
…ring This branch exists to make the docs describe the checkout, and one of the claims it added does not. README, the release plan and updates.md all present "offloader update finds a release" as working, and the version ordering section explains at length that prereleases are ordered rather than rejected. The endpoint underneath is /releases/latest, which GitHub documents as returning the newest published full release and excluding prereleases. The first packaged release is planned as 0.1.0b1, so an installed beta cannot discover its successor, and a repository holding only the betas the candidate workflow publishes with --prerelease answers with nothing at all. The ordering is right; the endpoint is wrong. All three places now say so and point at the correction pending on #13, and the release plan no longer counts discovery as a gate that can be signed off. Nothing here claims the fix: it states the limit as the checkout currently has it, which is what this branch is for.
owenpkent
left a comment
owenpkent
left a comment
There was a problem hiding this comment.
Follow-up review of 10da535, focused on the release-collection and channel-selection correction.
The previous beta-discovery finding is addressed in this head. Selection considers the collection, chooses the greatest eligible version rather than creation order, ignores drafts, allows prerelease installations to advance to a stable release, and keeps stable installations out of the prerelease channel.
Validation: all 8 focused collection/channel regressions passed. One integration requirement remains on child #15: its newly added check_feed() currently rejects a JSON list before calling this selector. The detailed finding is posted there, where that incompatible guard is introduced. This follow-up covers discovery; it does not claim the separately discussed installer-verification concerns have been resolved.
owenpkent
left a comment
owenpkent
left a comment
There was a problem hiding this comment.
No new nonduplicate public finding in this incremental diff. Previously private installer-verification concerns remain and are not resolved by the successful discovery tests.
Reviewed release collection/channel/version selection, asset matching, redirected streaming download, signature/version checks, CLI outcomes and installer handoff. The earlier beta-feed finding is corrected: discovery reads the release collection and selects the greatest eligible version while excluding drafts and respecting the stable/prerelease channel.
Validation: all 71 updater tests passed in a separate pinned source export. Current-head CI has successful matrix/build/frozen jobs plus older cancelled duplicates; the diff check passes. The new native installation-target finding is reported on parent #7, where that behavior originates.
Limits: mocked network/signature/launch boundaries, without downloading and executing a real signed release, hardware-key signing, UAC exercise or a clean-machine update. No-new-public-findings does not close the earlier private security work.
Pinned scope: declared base 6eda863bbc81630be6fe84f59d31af694ceb3500, merge base 6eda863bbc81630be6fe84f59d31af694ceb3500, head bf69fd36cd7923b6b09476963cb6d7250aa0fe7a. Existing discussion and current-head checks were reviewed; no threads were resolved and no formal approval or merge is being issued.
…hell path, honest failures The compare-before-launch check hashed nothing: verify was handed the same digest on both sides. The file is now re-hashed on disk before and after the signature probe and again immediately before launch, and a mismatch refuses. The signature verdict came from a bare "powershell" name, which Windows resolves through the current directory first. PowerShell is now run by its absolute System32 path with -NoProfile -NonInteractive from the system root, and the installer path reaches the probe through an environment variable rather than script text, so quoting cannot break or inject it. A feed that could not be read printed "newest release available" and exited 0; it now fails with exit 5. The installer's own refusal was never seen; the updater now waits where it can and reports the exit code, and an installed build that cannot wait exits 6 instead of claiming success. Downloads go to an exclusively created partial, are fsynced, promoted only after verification, and refuse linked directories. Non-installed builds are refused with a message. The certificate pin accepts an announced successor thumbprint so rotation does not strand installs.
# Conflicts: # README.md
# Conflicts: # README.md
1 participant
Summary
The installer landed in #7 with nothing to drive it. This is the update client the release plan's Updater boundary section described, and it meets the conditions that section set: signature, publisher and embedded version verified before elevation, and the
/D=target computed from the running executable rather than read from the uninstall registry key.Exit status distinguishes "nothing to do" (0) from "an update exists but was not applied" (1) from a verification failure (2), so it can be scripted.
Two deliberate divergences from alpha-osk
Worth reviewing first, because they are where I did not copy the reference:
taskkills the app and a helper process restarts it. Offloader's installer refuses maintenance while a transfer is in flight and exits non-zero, anddocs/data-safety.mdmakes that a promise. So this verifies and hands over. The command prints the consequence before the UAC prompt, because an operator who does not know it reads the installer's refusal as a broken update.X.Y.Ztags. Offloader's first packaged release is planned as0.1.0b1, so that rule would make the updater blind to the very build it ships in. The grammar here is the onebuild/windows/versioning.pyalready enforces, andtest_the_ordering_agrees_with_the_windows_version_fieldsasserts both orderings agree across a list of versions — a disagreement would let an update install a build Windows then considers older than the one it replaced.What it refuses
An updater runs network-delivered code with elevation, so the refusals are the design:
Content-Lengthand mid-streamValid+ our thumbprint + our publisher nameFileVersionmatches the releaselatestor1.0.3-evilbeing string-compared into an upgradeOffloader-Setup-{version}.exe/D=is emitted last and unquoted, the two rules NSIS enforces silently. Certificate constants are duplicated frombuild/windows/sign.pyrather than imported, since the build scripts are not shipped — flagging that as the one bit of duplication, in case you would rather move them into the package.Not included
download()takes aprogresscallback so the GUI can wire straight into it..github/workflows/release.ymlstill does not exist. Worth knowing: alpha-osk has no release automation either (releases are built andgh release created by hand), so this is net-new work rather than parity. Happy to do it next.Test plan
tests/test_update.py— 62 tests, mostly refusals: unparseable versions fail closed, prerelease ordering, the cross-check againstversioning.py, malformed feeds, wrong/renamed assets, non-HTTPS and off-allowlist hosts, a redirect to another host, the ceiling both ways, digest-changed-after-download, each signature rule failing on its own, thumbprint compared without spacing or case,/D=shape, and a declined UAC prompt reported as such rather than as a failed downloadruff check src tests scripts build/windowscleanoffloader updateagainst the real feed with no releases published returns "0.1.0 is the newest release available", exit 0 (fails closed on the 404 rather than raising)Review follow-ups (2026-10-12)
verifywas handed the same digest on both sides. The file is now re-hashed on disk before and after the signature probe and immediately before launch; a mismatch refuses.powershellname, which Windows resolves through the current directory first. PowerShell runs by its absolute System32 path with-NoProfile -NonInteractive, from the system root; the installer path reaches the probe through an environment variable, never script text..offloader-partial, are fsynced, and are promoted only after verification. Linked download directories are refused. Non-installed builds (pip, portable) are refused with a message.